chore: filtering move selector honoring the phase termination#1608
Conversation
triceo
left a comment
triceo
left a comment
There was a problem hiding this comment.
I am not sure that we want to do this.
To solve a pathological corner case, we slow down almost every move selector, filtering is used very often.
IMO the solution is worse than the problem.
I was also concerned about the performance. In the worst-case scenario, if all moves are filtered out, there will be no drop in performance. However, if no moves are removed, there will be two calls to check the termination. We could consider adding a triggering condition. For example, we would check that only if the filtered move count is equal to some value, such as 10% of the selector size. Also, if we can ensure the filtered selector is used in the move repository, we could remove the termination check from the decider implementation. |
triceo
left a comment
triceo
left a comment
There was a problem hiding this comment.
Some things to consider. LGTM after resolved.
zepfred
had a problem deploying
to
documentation (preview)
GitHub Actions
Failure
zepfred
had a problem deploying
to
documentation (preview)
GitHub Actions
Failure
|
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
2 participants
This PR ensures that the
FilteringMoveSelectorwill honor the phase termination setting. There may be use cases where a move filtering filters out all moves, resulting in the iterator analyzing a large number of moves before returning no valid upcoming selection.It is important to note that the proposed strategy may not work for every termination configuration, and the phase will only end after the move selector bails out. Termination not based on time can cause the process to stuck during the generation of the first valid move in the phase, which may result in no move being returned after bailing out.